test: rank individual and ensemble candidates with synthetic workload costs - #742
Open
milindsrivastava1997 wants to merge 117 commits into
Conversation
milindsrivastava1997
left a comment
milindsrivastava1997
left a comment
Contributor
Author
There was a problem hiding this comment.
Automated code review: 5 findings, posted inline.
milindsrivastava1997
force-pushed
the
732-test-add-prometheus-remote-write-promql-differential-suite
branch
from
September 21, 2026 17:58
1501e44 to
0b40465
Compare
milindsrivastava1997
marked this pull request as ready for review
September 22, 2026 03:19
zzylol
changed the base branch from
main
to
test/promql-exact-function-coverage
September 22, 2026 04:30
zzylol
changed the title
This was referenced Sep 22, 2026
This was referenced Sep 22, 2026
zzylol
changed the base branch from
test/promql-exact-function-coverage
to
issue-755
September 22, 2026 20:45
This was referenced Sep 22, 2026
zzylol
force-pushed
the
732-test-add-prometheus-remote-write-promql-differential-suite
branch
from
September 24, 2026 12:56
9c84758 to
00195cb
Compare
zzylol
changed the base branch from
test/promql-exact-function-coverage
to
fix/cost-evidence-dataset
September 28, 2026 18:15
…rite-promql-differential-suite' into impl/sds-stack-742
zzylol
added a commit
that referenced
this pull request
Sep 28, 2026
…needs issue754_workload is `#[path]`-included by every level's test binary, so each binary compiles the whole module while calling only the helpers that level needs. Under `-D warnings` that turns an unused helper into a hard error in whichever level does not call it: `certified_topk_input`, added for level 1's certified heap fixture, broke the level 2 binary in #742. The lint is about the including binary, not about the fixture, so allow it at the module level and say why. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Alignment with updated #737
The complete synthetic-ranking test passes. Besides making each admitted candidate win by changing prices, it rejects otherwise matching quotes from another dataset. Costs remain synthetic; this does not establish production-optimal selection.
Verify Backend candidate ranking independently of real cost calibration or execution correctness.
Before this PR: structural tests also selected winners with fixture costs, while #742 mixed in the differential execution harness.
After this PR: Level 2 uses the same ten-query workload and Planner-exposed candidate inventory as #728. Each unique admissible manifest becomes the cheapest candidate in turn. Before each round the test asserts every other candidate's synthetic total is strictly higher, so the preferred winner follows from construction rather than fixture multiplicities. Assertions cover selection reversal, minimum reported cost, exactly one winner, inventory-order stability, and exclusion of missing/infeasible quotes: the preferred candidate's own evaluation must report
EvidenceMissing(quote removed) orProviderRejected(executable = false). A single losing quote scoped to another dataset must reject the whole evidence withCompileError::CostEvidenceDataset(added in #780). No Planner logical winner is forced to construct the comparison.Validation:
issue754_level2passes across all ten queries; Clippy on the test targets adds no warnings (the existingcertified_topk_inputdead-code warning comes from #728's shared support file). All quotes are explicitly synthetic and establish ranking logic only. The final diff against #780 contains only the ranking integration test; the execution harness is preserved in #775.Current review order: #728 → #786 → #780 → #742 → #775. Workload + synthetic costs → selected Deployment Plan → data-plane execution is the current milestone. #776/#777/#778/#759 are deferred follow-ups, not prerequisites. Tie-breaking between equal-cost candidates and stale/future evidence are out of scope for this ranking test. Online ERP collection, feedback and runtime replanning are not required.
Query ensemble coverage
The fixtures now include each individual query, the shared-rate ensemble
(temporal-rate, grouped-rate, topk-rate), the shared-quantile ensemble
(temporal-quantile, quantile-ratio), and the full ten-query workload.
Level 2 makes each admitted workload candidate cheapest in turn using complete
synthetic quotes, asserts the exact selected manifest, reverses candidate order,
and rejects missing or infeasible cheapest quotes. The complete ranking test passed
for the ten single-query workloads and all three ensembles.